docs: Document GenericWebhook TaskSpawner source#1035
Merged
Conversation
github-actions
Bot
added
kind/docs
needs-triage
needs-kind
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Collaborator
|
/kelos review |
![kelos-bot[bot]](https://avatars.githubusercontent.com/in/2965305?s=60&v=4){kind=small}
Author
There was a problem hiding this comment.
🤖 Kelos Reviewer Agent @gjkim42
Review Summary
Verdict: COMMENT (would-block; cannot REQUEST_CHANGES on own bot-authored PR)
Overall correctness: patch is incorrect
Scope: Documents the existing spec.when.webhook (GenericWebhook) TaskSpawner source — README list, integration guide section, reference table rows, template-variable column, and a Sentry-driven example.
Findings Overview
| Priority | Count | File:Line | Summary |
|---|---|---|---|
| P0 | 0 | — | none |
| P1 | 2 | docs/integration.md:332, examples/13-taskspawner-generic-webhook/README.md:159 | Docs claim per-source HMAC signature validation that the handler does not actually perform |
| P2 | 0 | — | none |
| P3 | 1 | examples/README.md:25 | Example index jumps from 11 to 13 (pre-existing gap, but easy to fix while the table is being edited) |
Findings
Documentation accuracy / Security
- [P1]
docs/integration.md:332andexamples/13-taskspawner-generic-webhook/README.md:35-55,159— The Setup/Helm-secret instructions and the "The server validates each delivery against the matching<SOURCE>_WEBHOOK_SECRETusingX-Hub-Signature-256" / "Invalid signatures return HTTP 401" claims are not backed by the implementation. Ininternal/webhook/handler.gotheGenericSourceswitch branch (~lines 211–231) only extracts the source path, lists spawners, and computes a delivery ID — there is no signature-header read, no env-var lookup of<SOURCE>_WEBHOOK_SECRET, and no call to anyvalidateHMACSignaturehelper.signature.goexposes onlyValidateGitHubSignatureandValidateLinearSignature. The handler tests corroborate this:TestGenericServeHTTP_CreatesTaskForMatchingSpawnerandTestGenericServeHTTP_AcceptsUnknownSourceboth succeed with no signature header set, and there is noTestGenericServeHTTP_RejectsInvalidSignaturecounterpart. The Helm chart'senvFrom: secretRefmounts the secret into the pod, but no code reads it. As written, the docs lead operators to deploy a webhook endpoint they believe is HMAC-protected when it actually accepts any unauthenticated POST; this is a meaningful security misrepresentation, not a stylistic gap. Recommended fix: describe the current state honestly (the/webhook/<source>endpoint is unauthenticated, recommend restricting ingress / network policy) and file a follow-up issue to add<SOURCE>_WEBHOOK_SECRET-based HMAC validation. Alternatively, hold this PR until the validation is implemented in the handler.
Suggestions
- [P3]
examples/README.md:25— While editing the example index, consider also adding the existingexamples/12-taskspawner-file-patterns/row so the table doesn't skip from 11 to 13.
Key takeaways
- The docs make a security guarantee (HMAC signature validation) that the current generic webhook handler does not implement; following the Setup steps yields an unauthenticated webhook endpoint. Resolve before merge by either correcting the docs or wiring up the missing validation.
Collaborator
|
/kelos pick-up |
1 similar comment
Collaborator
|
/kelos pick-up |
Add user-facing docs for the spec.when.webhook (GenericWebhook) source type that has been merged but undocumented: - README.md: list Generic Webhooks (and Linear Webhooks) in the TaskSpawner integration summary. - docs/integration.md: add a Generic Webhooks section covering source, fieldMapping, filters, setup, and template variables; add a Generic Webhook column to the source-vs-variable table. - docs/reference.md: add spec.when.webhook.* rows to the TaskSpawner field reference and a Generic Webhook column to the promptTemplate variables table; mark workspaceRef as required for webhook sources. - examples/13-taskspawner-generic-webhook: add a Sentry-driven example with annotated YAML and a README walkthrough. Co-Authored-By: Claude Opus 4.7 <[email protected]>
The previous docs claimed the generic webhook endpoint validates each delivery against a per-source <SOURCE>_WEBHOOK_SECRET via X-Hub-Signature-256, but the handler does not currently read or verify any signature header — operators following the docs would deploy an endpoint they believed was HMAC-protected but that actually accepts any unauthenticated POST. Replace the HMAC-validation language with a security warning that describes the current state honestly, recommends restricting access at the network layer (NetworkPolicy / Ingress / Gateway IP allowlists), and links to the follow-up issue tracking the validation work. Note the chart's webhookServer.sources.generic.secretName field as reserved for future use. Also list examples/12-taskspawner-file-patterns/ in the examples index so it no longer skips from 11 to 13. Co-Authored-By: Claude Opus 4.7 <[email protected]>
kelos-bot
Bot
force-pushed
the
kelos-task-1033
branch
from
6a08ffe to
92104a0
Compare
Collaborator
|
/kelos review |
Author
|
🤖 Kelos Agent @gjkim42 Addressed the prior review's P1 finding (security misrepresentation about HMAC validation) and the P3 example-index gap in commit
|
Author
There was a problem hiding this comment.
🤖 Kelos Reviewer Agent @gjkim42
Review Summary
Verdict: COMMENT (would-approve; cannot approve own bot-authored PR)
Overall correctness: patch is correct
Scope: Documents the existing spec.when.webhook (GenericWebhook) TaskSpawner source — README list, integration guide section, reference table rows, template-variable column, and a Sentry-driven example under examples/13-taskspawner-generic-webhook/.
Findings Overview
| Priority | Count | File:Line | Summary |
|---|---|---|---|
| P0 | 0 | — | none |
| P1 | 0 | — | none |
| P2 | 0 | — | none |
| P3 | 0 | — | none |
Findings
None — the prior P1 finding (security misrepresentation around HMAC validation) and the P3 finding (examples/12-taskspawner-file-patterns/ missing from the index) were both addressed in commit 92104a0. Spot-checked claims against the implementation:
docs/integration.mdandexamples/13-taskspawner-generic-webhook/README.mdnow describe the endpoint as unauthenticated, recommend network-layer mitigations (NetworkPolicy/ Ingress IP allowlist / mTLS), note the chart'swebhookServer.sources.generic.secretNameenv-var mount is unread, and link the follow-up tracking issue #1040. This matchesinternal/webhook/handler.go(theGenericSourcebranch at lines 211–231 reads no signature header and calls no validator) andinternal/webhook/signature.go(onlyValidateGitHubSignature/ValidateLinearSignatureexist).- The
fieldMapping/filterssemantics —idrequired, JSONPath against the parsed body, missing fields → empty strings, malformed JSONPath → error, AND-across-filters, missing filter field → silent skip, lowercaseid/title/body/urlaliased to uppercase,{{.Kind}} = "GenericWebhook",{{.Payload}}is the full body — all line up withinternal/webhook/generic_filter.go(ExtractFields,MatchesGenericFilters,ExtractGenericWorkItem,extractGenericDeliveryID) and the API-levelXValidationrule inapi/v1alpha1/taskspawner_types.go:471. docs/reference.mdcorrectly extendsworkspaceRef.name's required-when clause towebhookand adds the fivespec.when.webhook.*rows; theGeneric Webhookcolumn in the template-variable table matches whatExtractGenericWorkItempopulates (Number / Labels / Comments / Event / Action / Sender / Branch / Ref / Repository* / Review* / Type / State / IssueID / Time / Schedule are all empty for this source).examples/README.mdno longer skips from 11 to 13.
Key takeaways
- Docs-only change, accurate against the current implementation. Earlier review threads from kelos-bot have been resolved as obsolete; this PR is ready to merge from a docs-correctness standpoint.
kelos-bot Bot
pushed a commit
that referenced
this pull request
Apr 30, 2026
Add a project convention and reviewer checklist item that documentation should describe only what the code actually does, not aspirational behavior. Evidence: - PR #1035 (kelos-bot[bot] P1): docs claimed HMAC signature validation per source, but the GenericSource branch in handler.go never reads the webhook secret, never inspects X-Hub-Signature-256, and never calls validateHMACSignature — giving users a false sense of security. - PR #1056 (kelos-bot[bot] P2): doc note claimed PodOverrides.Env entries reusing reserved names are dropped so the built-in always wins, but the actual filter only drops collisions against names already populated on mainContainer.Env, leaving KELOS_SETUP_COMMAND passable when the field is empty. Applied to AGENTS.md (symlinked to CLAUDE.md), agentconfig.yaml, kelos-workers.yaml, and a new "Documentation accuracy" subsection in the kelos-reviewer.yaml checklist. Co-Authored-By: Claude Opus 4.7 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
/kind docs
What this PR does / why we need it:
The
spec.when.webhook(GenericWebhook) TaskSpawner source has been merged and tested but is entirely undocumented, which forces new users to write custom glue code instead of finding the feature. This PR adds user-facing documentation for it.Specifically:
README.md— list Generic Webhooks (and the previously-omitted Linear Webhooks) in the TaskSpawner integration summary.docs/integration.md— add a Generic Webhooks section coveringsource,fieldMapping,filters, setup, and template variables; add a Generic Webhook column to the source-vs-template-variable table; document that arbitraryfieldMappingkeys also become template variables.docs/reference.md— addspec.when.webhook.{source,fieldMapping,filters[].field,filters[].value,filters[].pattern}rows to the TaskSpawner reference table; add a Generic Webhook column to thepromptTemplatevariables table; markworkspaceRef.nameas required for the webhook source too.examples/13-taskspawner-generic-webhook/— new example illustrating a Sentry-error-driven TaskSpawner with annotated YAML and a README walkthrough (setup, field mapping, filters, sample payload, troubleshooting).examples/README.md— also list12-taskspawner-file-patterns/so the index no longer skips from 11 to 13.Security note
The generic webhook endpoint does not currently validate request signatures — the API doc-comment, the Helm chart's
webhookServer.sources.generic.secretNamefield, and the<SOURCE>_WEBHOOK_SECRETenvFrom mount all imply HMAC protection that the handler does not implement. The docs describe this state honestly, recommend restricting access at the network layer (NetworkPolicy / Ingress IP allowlists / mTLS), and link to the follow-up issue tracking the missing validation.Follow-up: #1040
Which issue(s) this PR is related to:
Fixes #1033
Special notes for your reviewer:
Docs-only — no behavior changes.
make verifyandmake testpass.Does this PR introduce a user-facing change?